Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CURA-9178] Port external Voronoi fixes #1731

Closed
wants to merge 9 commits into from

Conversation

rburema
Copy link
Member

@rburema rburema commented Sep 12, 2022

Workarounds and fixes for certain Arachne foibles. Mostly to do with crashes due to improper Voronoi generation by Boost (or pre-processing necessary for that).

See individual commit messages for greater explanation.

We received a mail from someone that pointed us to these extant fixes. Would be a shame if we didn't incorporate them ;-)

--> Currently in draft, as it only compiles, not tested yet at all!

--> Also there are some problems, see the discussion(s) below. My attempt(s) to fix these went beyond a port (but are simplified as well in way) and can be found in the CURA-9178_temp_sidebranch.

rburema and others added 5 commits September 12, 2022 21:45
First of the externally contributed Voronoi fixes/workarounds. Boost-Voronoi doesn't always correctly generate the Voronoi we need. One of the problems is that a vertex we know should be there (from the polygon) can be missing. As the possibility of fixing it within Boost (or even another Voronoi solution or library) seems distant, if we detect a case like this, we rotate the model and try again. (This also forms the basis for at least one other workaround. -- See next commit in this PR probably.)

Co-authored-by: Lukáš Hejl <[email protected]>
Second of the externally contributed Voronoi fixes/workarounds. Boost-Voronoi doesn't always correctly generate the Voronoi we need. The bigger of the (at least two) problems is that the Voronoi-diagram can intersect itself in strange ways (AKA be 'non-planar' in graph-theoretical terms). As the possibility of fixing it within Boost (or even another Voronoi solution or library) seems distant, if we detect a case like this, we rotate the model and try again. (This is based on the basework laid in the previous commit.)

Co-authored-by: Lukáš Hejl <[email protected]>
Reported externally by others using the Arachne code in their engine. Not checked if this actually happens in CuraEngine yet as well, but can't hurt as it shouldn't alter the outcome if already correct.

Co-authored-by: Lukáš Hejl <[email protected]>
Reported externally by others using the Arachne code in their engine. Not checked if this actually happens in CuraEngine yet as well, but seems plausible, and it's only an epsilon otherwise.

Co-authored-by: Lukáš Hejl <[email protected]>
Purely deensive programming, as CuraEngine (probably) doesn't use large enough scales for this to _actually_ trip up the boost voronoi generation, yet. But it helps to be prepared.

Co-authored-by: Lukáš Hejl <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2022

Unit Test Results

25 tests  ±0   25 ✔️ ±0   13s ⏱️ +5s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ±0 

Results for commit 42182a2. ± Comparison against base commit 36e778f.

♻️ This comment has been updated with latest results.

@Piezoid
Copy link
Contributor

Piezoid commented Sep 13, 2022

This looks very interesting as a solution for this long standing issue!

However I can't slice simple models with it. After Detected non-planar Voronoi diagram even after the rotation of input., it segfaults trying to access a point on an polygon that was destroyed.

rotate_fail.zip

Otherwise dangling references result in vertex_mapping and/or segments, since those can refer to the vertices in the polygon copy.
@Piezoid Piezoid mentioned this pull request Oct 7, 2022
2 tasks
@rburema
Copy link
Member Author

rburema commented May 21, 2024

This never worked properly for us, and we've got a completely different workaround by using the Wagyu library to fix our polygons instead. (See: #2030 )

@rburema rburema closed this May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants